-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Pending umbrelOS feature] - App Submission: Omada Controller #1196
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @hugofnm for your awesome submission! I have left some suggestions for this PR. Sorry that you had to wait so long, but we have just finished our linter for any new app submissions. You will see it when you are adding new commits to this PR :)
Thanks @sharknoon for the suggestions ! |
Thanks for the ping and review @sharknoon 🤝. And great work packaging Omada Controller @hugofnm! The only outstanding task is to avoid potential port clashes with Omada Controller running in host networking mode. It’s on the umbrelOS roadmap to mitigate port clashes automatically, but this currently isn't implemented, and port This means if a user tries to run both Omada Controller and Element on the same device, there will be a port clash, causing one of the apps to fail to launch (or fail to install). To mitigate this, here are the ports Omada Controller uses: We don’t need to worry about UDP ports, as they can be shared. The only one we need to plan for right now is Here are the env vars we have access to: So we can use |
Hi @nmfretz 👋, Looking at the official documentation (https://www.tp-link.com/us/support/faq/3281/), it seems that Omada Controller uses about 10 different TCP ports for device discovery, web management, ... Also, should this app stay on host mode or should I expose each port individually? |
🎉 Linting finished with no errors or warnings 🎉Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store. Please review the linting results below and make any necessary changes to your submission. Linting Results
Legend
|
Thanks for doing that @hugofnm. With this port change, let's leave this in host network mode because Omada will need this for device discovery. Gallery assets are here: https://github.com/getumbrel/umbrel-apps-gallery/tree/master/omada-controller @hugofnm @sharknoon, I have made some small tweaks and have tested the app. One thing to look into before this goes live is the ssl cert experience. Currently, users who install Omada Controller will be met with a warning in their browser that the connection is not secure. @hugofnm do you happen to know whether we can avoid this entirely via compose configuration? We should really try to avoid encouraging users to click past these warnings, but as a last resort we could include instructions in the app's description. |
@hugofnm Is there a way to disable the HTTPS endpoint completly and only use the HTTP one? |
Well... I'm afraid it's not possible to disable the HTTPS endpoint @sharknoon |
Darn, that's a shame. This means that the flow for a user installing Omada Controller would be:
Great work packaging this app @hugofnm. The effort is super appreciated. Unfortunately, I think we should wait to bring this to the official app store until after umbrelOS has integrated HTTPS support, which is on the horizon. That way users of all technical skill levels can easily use the app and we aren't providing a poor experience as a result of the OS. Would you prefer that we convert this PR to draft but keep it open, or would you prefer to close this PR and reopen once HTTPS support is available? In the meantime people could still run Omada Controller on their Umbrel by either:
|
Sure, I'll convert this PR to draft until Umbrel supports HTTPS ! |
Sounds good, thanks @hugofnm 🤝 |
Reminder for us: gallery assets have already been prepared for Omada Controller |
App Submission
App name
Omada Controller
256x256 SVG icon
(Submit an icon with no rounded corners as it will be dynamically rounded with CSS. GitHub doesn't allow uploading SVGs directly, so please upload your icon to an alternate service, like https://svgur.com, and paste the link below.)
We will help finalize this icon before the app goes live in the Umbrel App Store.
https://www.tp-link.com/res/images/icons/tag-omada.svg
Gallery images
(Upload 3 to 5 high-quality gallery images (1440x900px) of your app in PNG format, or just upload 3 to 5 screenshots of your app and we'll help you design the gallery images.)
We will help finalize these images before the app goes live in the Umbrel App Store.
I have tested my app on: